We haven't introduced any checks to skip from checking for clang tidy. Probably we can take the ones from PGM, most of clang tidy complaints should be fixed that way.

We haven't introduced any checks to skip from checking for clang tidy. Probably we can take the ones from PGM, most of clang tidy complaints should be fixed that way.

I propose only disabling the checks that are disablef in pgm and that also complain on this repo. New code should be better bu default, and case-by-case disabled at worst

I believe sonar cloud complaints are in a way false positive.
Complains for const are there just because our functions are empty, after we fill them in, we should decide whether they need constness or not. I do not think that there is a point to add constness to functions that might not be const.

Complaints for new and delete are also not accurate I think. It would prefer if we used memory management techniques which automatically manage the memory, but they free up the memory when going out of scope. So if the scope is a function where we created the pointer, after that function it would be destroyed. While in our example, we have 3 api functions which all need the same pointer.

I can't add to much towards arguing for reinterpret_cast, but it is used when casting from one pointer to another, useful in opaque pointers.

I believe sonar cloud complaints are in a way false positive. Complains for const are there just because our functions are empty, after we fill them in, we should decide whether they need constness or not. I do not think that there is a point to add constness to functions that might not be const.

You are right. Feel free to disable those false positives if you are certain.

Complaints for new and delete are also not accurate I think. It would prefer if we used memory management techniques which automatically manage the memory, but they free up the memory when going out of scope. So if the scope is a function where we created the pointer, after that function it would be destroyed. While in our example, we have 3 api functions which all need the same pointer.

Same goes here.

Complains for const are there just because our functions are empty, after we fill them in, we should decide whether they need constness or not. I do not think that there is a point to add constness to functions that might not be const.

That is fair. It can be re-evaluated when the actual implementation is made.

Complaints for new and delete are also not accurate I think. It would prefer if we used memory management techniques which automatically manage the memory, but they free up the memory when going out of scope. So if the scope is a function where we created the pointer, after that function it would be destroyed. While in our example, we have 3 api functions which all need the same pointer.

This may be resolved once the actual implementation of the VNF converter class is implemented, as the memory management can be dealt with there in a safer way. For now, it should be okay.

I can't add to much towards arguing for reinterpret_cast, but it is used when casting from one pointer to another, useful in opaque pointers.

There is nothing much we can really do here, that is how C APIs work.

On the other hand, when we leave so many un-resolved smells on purpose, do we have a way to go about it at the moment? This can be easily forgotten, or at least the reasoning behind it. Maybe @Jerry-Jinfeng-Guo or @mgovers can comment on this?

On the other hand, when we leave so many un-resolved smells on purpose, do we have a way to go about it at the moment? This can be easily forgotten, or at least the reasoning behind it. Maybe @Jerry-Jinfeng-Guo or @mgovers can comment on this?

@figueroa1395 The priority now is push forward this project. Most of the sonar cloud issues are indeed false positive if we inspect them case by case. Being stuck at polishing at such an early stage is not only counterproductive but can also be frustrating for @Laurynas-Jagutis . Therefore, I would argue for pushing forward so long as the direction is correct.

@figueroa1395 The priority now is push forward this project. Most of the sonar cloud issues are indeed false positive if we inspect them case by case. Being stuck at polishing at such an early stage is not only counterproductive but can also be frustrating for @Laurynas-Jagutis . Therefore, I would argue for pushing forward so long as the direction is correct.

I agree. And it should not be a blocker specially at this stage. Maybe I phrased my words wrong. I was just raising it such that we can create an action point to make agreements (if not present yet) to know how to handle it in the future. But I think we can just ignore them for this PR.

Quality Gate passed

Issues
19 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

I can't add to much towards arguing for reinterpret_cast, but it is used when casting from one pointer to another, useful in opaque pointers.

There is nothing much we can really do here, that is how C APIs work.

On the other hand, when we leave so many un-resolved smells on purpose, do we have a way to go about it at the moment? This can be easily forgotten, or at least the reasoning behind it. Maybe @Jerry-Jinfeng-Guo or @mgovers can comment on this?

yeah.... this has been on my mind as well... Personally, I think we should use either char* or std::byte* internally instead of void* for anything other than the C API itself. Then, all our reinterpret_casts can be contained in a single location in the C API and no-where else. This would also remove all ambiguity regarding the sizeof(void) vs sizeof(char) vs sizeof(<Object>).

This is a team decision, though.

This is a team decision, though.

Maybe we can bring this up as an action point to discuss in a knowledge sharing session. Definitely we should try to contain reinterpret_cast within the C APIs. It is too dangerous to have it in as many places as we do now.